Introduce deprecated annotations#6
Conversation
f846aa8 to
dd0006a
Compare
|
Hmm, clever, but I don't think this is the right pattern. Deprecation/transitions are typically done as annotation only, without changing validation behavior. Common three-phase lifecycle:
If we collapse 2+3, you run into a host of issues... Announcing intent IS the breaking changeThe moment you add a transition annotation to a Schema contradicts its own documentationThe PR docs say for Tightening transitions are invisibleFor Comparables..
|
|
All that said, I think the general shape is sound. A few tweaks I'd suggest... Explicit "ucp_request": {
"update": {
"transition": { "from": "required", "to": "omit", "description": "Use resource_id instead." }
}
}Shorthand: "ucp_request": {
"transition": { "from": "required", "to": "omit", "description": "Use resource_id instead." }
}
ResolutionThe resolver uses
{
"id": {
"type": "string",
"description": "The resource identifier.\n\n⚠ Transition: Removing in v2. Use resource_id instead.",
"x-ucp-transition": { "from": "required", "to": "omit", "description": "Removing in v2..." },
"deprecated": true
}
}LifecycleAnnounce: behavior unchanged, tooling warns "ucp_request": {
"update": {
"transition": { "from": "required", "to": "omit", "description": "Removing in v2." }
}
}
→ resolves as required
→ emits x-ucp-transition + deprecated: trueExecute: change when ready "ucp_request": { "update": "omit" }
→ field removed from resolved schema |
|
@igrigorik thank you for the feedback! I'm happy to change the PR in this direction, but I want to:
Tradeoff: Will not block on this tradeoff, I do think your proposal is also a solid perspective and we do not have to optimize for how folks are applying our schemas; your proposal is a valid schema! Just calling out the trouble some folks in the community will face. Blocking question: and to and Could you help me understand the value prop from this change? My initial reaction is just seeing additional nesting that I'm not sure is needed, but please lmk what I'm missing! |
10095e8 to
6baae26
Compare
|
Thanks for the feedback @igrigorik, we should be good to go now! ♻️ 🚀 And to summarize an async convo we had to unblock my comments above:
|
igrigorik
left a comment
There was a problem hiding this comment.
overall, lgtm, but let's align behavior across resolve and validate code paths.
06e789f to
5170d8f
Compare
igrigorik
left a comment
There was a problem hiding this comment.
lgtm, modulo outdated comment
c5f5831 to
f3e3f3d
Compare
| diagnostics.push(Diagnostic { | ||
| severity: Severity::Error, | ||
| code: "E004".to_string(), | ||
| code: "E005".to_string(), |
There was a problem hiding this comment.
Is this intentional? Did something become slightly more-or-less severe?
There was a problem hiding this comment.
I think you're just seeing the artifact of the code-shuffle here Drew!
"invalid {} value "{}": expected omit, required, or optional" is staying as E004.
E005 can occur in two different ways now: either the top-level obj is failing to be a string or transition obj, or the sub-obj failed to be it.
605baec
into
Universal-Commerce-Protocol:main
Description
Adds first-class support for field deprecation with explicit schema transition semantics. This enables schema authors to signal upcoming breaking changes while attempting to preserve backwards compatibility during the transition period.
The schema now adds "deprecated" for transitions to "omit" visibility, and a x-ucp-schema-transition field to express the upcoming changes in an expressive way inside each schema object.
Examples
Required -> Optional
Input:
Output:
Required -> Omit
Input:
Output:
Motivation
When evolving APIs, transitions like
required → optionalorrequired → omitare breaking changes that affect different parties:required → optional): Receivers must update to handle absenceoptional → required): Senders must update to always include the fieldPreviously, there was no way to express "this field is currently required but will be removed" in a machine-readable way.
Changes
Introduces schema-transitions object which describes the current state, the future state, and a description for the change. Readme describes the expectation for implementation based on each valid transition. Each transitioning visibility is treated as optional while between states, allowing the sender and receiver of each request to properly handle absence without typing errors (someone is always looser than the other).
The concern here: even introducing a schema-transition WILL ALWAYS BE BREAKING because the schema itself will have to loosen in a breaking way for at least one party. Each of businesses or platforms will have to re-parse the output schema state based on the x-ucp tag to identify what fields should or should not be required, optional, or omitted for them truthfully in a version.
Why do we need more than a simple "deprecated" field? Why do we have states?
UCP annotations are symmetrical by default between the sender and receiver; we are introducing integration burdens on just one party for a given request/response (i.e. sender of request, please start ALWAYS sending it as its becoming required, or receiver of this response, stop expecting it even though it was required).